Skip to content

Allow Clippy to define duplicate extra symbols #143596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow Clippy to define duplicate extra symbols #143596

wants to merge 1 commit into from

Conversation

Alexendoo
Copy link
Member

Fixes #142410 (comment)

rustc_macros::symbols now also expands to a macro extra_symbols that calls rustc_macros::extra_symbols_impl with the prefill stream so that it can know which symbols are duplicated

extra_symbols allows supplying a name to define a macro to provide even more extra symbols, this is useful for external drivers that rely on clippy_utils e.g. TheBevyFlock/bevy_cli#456

There's a usage example in compiler/rustc_span/src/symbol/tests.rs

DUPLICATE_SYMBOLS is currently unused, but we can use it on the clippy side to remove duplicates without interfering with r-l/rust PRs

@rustbot
Copy link
Collaborator

rustbot commented Jul 7, 2025

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jul 7, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 7, 2025

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Alexendoo Alexendoo force-pushed the clippy-allow-duplicate-symbols branch from 393f4ae to aebfd04 Compare July 7, 2025 20:02
flip1995
flip1995 approved these changes Jul 8, 2025
View reviewed changes
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clippy changes LGTM

@fee1-dead
Copy link
Member

fee1-dead commented Jul 11, 2025 •
edited
Loading

This seems to be quite above my personal threshold for the compiler to maintain and support this, since it isn't a mechanism used by the compiler itself. Do you think there could be an easier solution here? For example, rustc may expose the number of symbols, then clippy could scan the rustc list (either the file itself, or exposed through a constant) to error at compile time if there is a duplicate symbol.

Or clippy could use a LazyLock.

@fee1-dead fee1-dead added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 11, 2025
@bors
Copy link
Collaborator

bors commented Jul 25, 2025

☔ The latest upstream changes (presumably #144449) made this pull request unmergeable. Please resolve the merge conflicts.

@fee1-dead fee1-dead mentioned this pull request Sep 9, 2025
Merged
@Alexendoo
Copy link
Member Author

An update on this:

clippy could scan the rustc list (either the file itself, or exposed through a constant) to error at compile time if there is a duplicate symbol.

This works with the new const PartialEq but currently it's a too slow to do the linear scans. With const PartialOrd from #143800 we could do a binary search so I'll mark this as blocked for now

In the meanwhile #146347 improves the runtime error

@Alexendoo Alexendoo added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flip1995 flip1995 flip1995 approved these changes

Assignees

@fee1-dead fee1-dead

Labels
S-blocked Status: Blocked on something else such as an RFC or other implementation work. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Alexendoo @rustbot @rust-log-analyzer @fee1-dead @bors @flip1995